Skip to content

perf(gdn): optimize post-conv and causal-conv kernels - #155

Open
richiejp wants to merge 6 commits into
mudler:mainfrom
richiejp:row/KERNEL-SSM-MAMBA-POSTCONV-TILE
Open

perf(gdn): optimize post-conv and causal-conv kernels#155
richiejp wants to merge 6 commits into
mudler:mainfrom
richiejp:row/KERNEL-SSM-MAMBA-POSTCONV-TILE

Conversation

@richiejp

@richiejp richiejp commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Row

KERNEL-SSM-MAMBA — Qwen GDN post-conv and causal-conv kernel sublevers; one row per PR.

Before starting

  • Issue/PR search and existing claim: this is a stacked follow-on to perf(gdn): dispatch exact causal-conv chunks #127, which supplies the exact causal-conv chunk descriptors measured here. No separate matching open PR was found for these two opt-in kernel specializations.
  • Roadmap or matrix row, plus scripts/ready-for-helper.py result when applicable: .agents/kernel-matrix.md:157 (KERNEL-SSM-MAMBA, INVENTORIED), feeding ROAD-V1-C2-LOCAL-BF16. Spikes: .agents/specs/sm120-qwen35-postconv-token-tile-2026-08-08.md and .agents/specs/sm120-qwen35-conv-channel-tile-2026-08-08.md.
  • Exact current-code and test/evidence anchors inspected: src/vt/cuda/cuda_gdn.cu, src/vt/cuda/gdn_prefill_conv.h, tests/vt/test_gdn_prefill_conv.cpp, tests/vt/test_ops_gdn.cpp, and pinned vLLM/FLA causal-conv plus fused post-conv kernels recorded with file:line anchors in the two spikes.

What changed

Add two independently selectable, default-off CUDA specializations. VT_GDN_POSTCONV_TOKEN_TILE=1 ports the upstream-shaped 16-token/four-warp post-conv schedule while preserving the existing 128-lane reduction tree byte-for-byte. VT_CONV_CHANNEL_TILE=1 specializes the causal-conv path for width four at the existing one-channel-per-thread grid; arm =2 is retained as the measured 256-channel/two-channels-per-thread falsification arm. Production dispatch and the portable mutation gate share one callback dispatcher, so the tests prove that each selected arm is actually invoked.

Evidence

  • scripts/agent-preflight.sh passes after rebasing onto canonical upstream/main c05cee1d2; all record gates, mutation suites, live-row audit, and committed-range document checks are green.
  • Tests covering this change: contained CUDA rebuild including an explicit vllm-bench relink; portable dispatch 9/9 cases, 88/88 assertions; CUDA GDN 67/67 cases, 4631/4631 assertions; Qwen3.5 paged-forward 4/4 cases, 8/8 assertions. The accepted production arms produce identical token hashes.
  • Same-change doc obligations: docs/STATUS.md and docs/BENCHMARKS.md updated; the model/feature surface did not expand. Full profiler evidence and the void stale-link series are retained in the two specs and .agents/benchmark-record.md.

Speed claims

  • The operator ran the numbers under ${GPU_LOCK} and they are recorded in docs/BENCHMARKS.md with the repro recipe.

On the local RTX 5070 Ti / Qwen3.5-4B c32 workload, the post-conv tile reduces graph-node time 227.887 to 122.587 ms (1.859x) and improves every observed enclosing axis, including total/output throughput by 0.532%. The causal-conv K4 arm reduces graph-node time 234.605 to 219.506 ms (6.44%) and improves whole-run total/output throughput by about 0.12%. The 256-channel arm is 4.05% slower than the K4 arm and is recorded as falsified.

Honest gaps

  • This PR is stacked on perf(gdn): dispatch exact causal-conv chunks #127. Until perf(gdn): dispatch exact causal-conv chunks #127 merges, GitHub shows its exact-chunks commits in this PR too; after that merge, this PR reduces to the opt-in follow-on commits.
  • Both specializations remain opt-in pending repeated A/B and hardware-unavailable 27B/35B gates. The 4B result is not extrapolated to the release models.
  • Same-tool residuals remain: post-conv is 1.135x and causal conv K4 is 1.509x slower than pinned vLLM.
  • The first production causal-conv profile series was void because vllm-bench had not relinked the rebuilt CUDA library. The accepted series explicitly relinked the benchmark and trace-proved the specialized kernels.
  • Current main c05cee1d2 independently fails check-device-leakage.py: src/vllm/v1/worker/gpu/runner.cpp contains one new hard-coded DeviceType::kCUDA (DSR kcuda 1 > baseline 0). perf(gdn): dispatch exact causal-conv chunks #127 and this PR inherit that merged-tree failure; neither kernel diff introduced it.

mudler added a commit that referenced this pull request Aug 8, 2026
…re-push sandbox (#159)

Two guards on main were RED and between them blocked every open external
contributor PR (#127, #154, #155) and every push. Both premises were verified in
the tree before changing anything.

1) check-device-leakage: src/vllm/v1/worker/gpu/runner.cpp named
   vt::DeviceType::kCUDA in the device-agnostic shared layer (DSR bucket 'kcuda'
   1 > baseline 0). It came in with the QueueSupportsAsyncInputCombine rescope
   during the PR #140 fix round - ours, not the contributors'; richiejp reported
   it in #127's honest gaps. Fixed the way the guard's own message prescribes,
   mirroring the SupportsAuxStream precedent: ask the backend, not the device.
   New vt::Backend::SupportsAsyncSampledTokenReadback() (base false) answers
   whether the host may validly read the sampled token id back between steps;
   CPU overrides true (host and device memory are one allocation) and CUDA
   overrides true (the id is device-mirrored). The runner asks
   vt::TryGetBackend(queue.device.type), whose nullptr for a device absent from
   the build also subsumes the old #ifdef VLLM_CPP_CUDA guard.
   SEMANTICS UNCHANGED: CPU async-ON, CUDA async-ON, discrete non-CUDA (ROCm
   gfx1201) async-OFF - the "!"-token hazard stays closed.

2) .githooks/pre-push ran check-policy.py inside a PARTIAL export
   (README.md docs scripts .agents), but policy_contract.py:428 asserts AGENTS.md
   is a non-symlink regular file and resolves its Markdown links against that
   sandbox. AGENTS.md and its .env.example link were both missing, so the hook
   failed closed on content that is fine in the real tree - every push refused.
   EXPORT_PATHS is now a superset of what the CHECKERS read.

Gates: check-device-leakage RED->GREEN (kcuda=0, DSR 32 == baseline 32); all four
hook checkers OK in the reproduced sandbox; test_async_llm 8/8-347,
test_engine_core 6/6-44, test_llm_engine 11/11-204 (CPU still resolves async-ON);
clean -Werror CPU build; full 11-gate record battery green.

FOLLOWING_AGENTS_PROTOCOL
Assisted-by: Claude (Opus 5) via Claude Code

@localai-bot localai-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Read through this one on top of #127. The shape is right and I want it, with one sequencing note.

What I like:

  • Both arms are genuinely opt-in. ConvChannelTileArmFromEnv accepts only the exact strings 1 and 2; unset, 0, and every other spelling keep arm 0, and non-K=4 shapes fall back regardless. GdnPostConvTokenTileFlagIsOn likewise keeps the byte-identical megablock default. That is the conservative default discipline we ask for, and the comments say plainly that a microbenchmark win implies no default flip.
  • Putting the arm selection in one ConvChannelTileLaunchContractFor seam shared by the CUDA launcher and the tests is the right call — the tests exercise the production branch logic instead of a parallel copy, with no launch counter or debug state in the hot path.
  • VT_CONV_CHANNEL_TILE is documented in docs/ENVIRONMENT.md with the fallback semantics spelled out, and both spikes are written up under .agents/specs/.

Sequencing: this is stacked on #127, so it lands after it. Its pr-size and documentation-checkpoint failures are largely inherited from the stack; once #127 is in, please rebase so this PR shows only its own delta and we can judge those two on their merits.

Not a blocker, a question: since both arms stay off until same-binary correctness and performance gates close — what would close them for you? If it is a GB10/sm_121 A/B I can run it here; if it needs sm_120 specifically, say so and I will note the arms as measured-on-sm_120-only in the kernel matrix so nobody later mistakes "opt-in" for "unproven".

Same note as on #127: agent-record is currently red repo-wide because .agents/policy-cutover points at a pre-squash SHA that never landed on main. Not yours.

@localai-bot

Copy link
Copy Markdown
Collaborator

Reviewed on GB10 (sm_121a, full production build: CUTLASS + FA2 + vendored Triton-AOT). The structure of this PR is exactly right — committed spikes with upstream anchors, two independently selectable default-off arms, and a retained falsification arm. Most of the red CI is staleness. But there is one real blocker, and it is in the new work.

Blocker: a 0-ulp equivalence regression that main does not have

TEST CASE: CUDA causal_conv1d_fwd register kernel (VT_CONV_REG) matches tiled 0-ulp
tests/vt/test_ops_gdn.cpp:726: ERROR: CHECK( bad == 0 ) is NOT correct!
  values: CHECK( 2661 == 0 )
  logged: c := 1024,  k := 4,  cb.in := 0,  cb.out := 0,  row_padding := 0

Side by side, same box, same build flags, same command:

tree test_ops_gdn
current main (0cf3dbbb) 66 cases, 4300/4300, SUCCESS
this branch rebased on main 67 cases, 4630/4631, 1 FAILED

c=1024, k=4 is precisely the shape VT_CONV_CHANNEL_TILE specialises, and 2661 mismatching elements is not a rounding tail — it is a wrong result, at default settings, on a test that asserts bit equality. Note this fires with the arms unset, so it is not confined to the opt-in path.

Worth checking whether the specialisation changed what the "tiled" reference computes for k=4, rather than only what the register kernel computes — if the reference moved, the test is comparing two new things and the 0-ulp claim in the spike no longer holds.

What I verified, so you don't have to re-do it

Rebasing onto main clears three of the four red checks by itself:

  • device-leakage — not your bug. Your branch predates 1f3b7148, which replaced the raw queue.device.type == kCUDA test in QueueSupportsAsyncInputCombine with the Backend::SupportsAsyncSampledTokenReadback() capability. Rebased, the DSR bucket is clean.
  • pr-size — errors with base must be an ancestor of head, pure staleness. Passes after rebase.
  • agent-record — green on current main.

And the default-off contract holds, which is the important safety property:

default (arms unset)        27B 235/235   35B 315/315   (SACRED)
VT_GDN_POSTCONV_TOKEN_TILE=1  27B 235/235
VT_CONV_CHANNEL_TILE=1        27B 235/235
test_gdn_prefill_conv         88/88

So both arms are token-safe on the 27B gate; the problem is confined to the conv equivalence test above.

Still owed after that

  • doc-checkpoint is per-commit: commits 66087699 and the final record commit change .agents/benchmark-record.md without touching .agents/NOW.md. Both need the NOW row in the same commit.
  • STATUS ratchet slacktest_the_live_page_keeps_the_character_ratchet_tight wants slack <= 25; re-pin chars to the measured value in the same change.

Heads-up for the rebase: main rewrote AGENTS.md and deleted the structured state record (.agents/state.csv, state-events/, state-index/), so drop any edits to those — narrative lives in the commit message now. I hit a conflict there myself and briefly re-added a 7k-line archived blob before catching it.

I haven't pushed the rebase or touched the code — findings go back to you rather than getting repaired in review. Fix the conv equivalence and the two doc gates and I'll re-run this whole set and merge.

@richiejp
richiejp force-pushed the row/KERNEL-SSM-MAMBA-POSTCONV-TILE branch 3 times, most recently from b3f019a to a651838 Compare August 10, 2026 13:56
Select the measured sm_120 post-conv residual, reject the existing split grid, and bind the upstream 16-token CUDA experiment and gates.

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: Codex:gpt-5.6-sol [Codex]
Port the upstream 16-token per-head schedule behind an opt-in flag, preserve the existing reduction order byte-exactly, and record the sm_120 A/B evidence.

Row: row/KERNEL-SSM-MAMBA-POSTCONV-TILE

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: Codex:gpt-5.6-sol [Codex]
FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: Codex:gpt-5.6-sol [Codex]
FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: Codex:gpt-5.6-sol [Codex]
Route runtime-width and both K=4 CUDA launch arms through the same portable, mutation-tested callback dispatcher. Pin strict arm-2 parsing and resolved non-K4 fallback semantics.

row/KERNEL-SSM-MAMBA-POSTCONV-TILE

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: Codex:gpt-5.6-sol [Codex]
FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: Codex:gpt-5.6-sol [Codex]
@localai-bot

Copy link
Copy Markdown
Collaborator

Merged, with two fixes applied during landing and one new issue filed. Thanks — the kernels themselves reviewed clean: no OOB index, no shared-memory overrun, no divergent barrier, no missing tail guard, and no launch-config assumption the launcher doesn't guarantee. I also verified the token-tile reduction-identity claim bitwise: the (q0²+q2²)+(q1²+q3²) fold really is the shipped 128-lane tree's first two levels for the lane + item*32 mapping — 0 of 20,000 random f32 rows differ. And the portable-header suite is genuinely strong: 9 cases / 88 assertions green, with 8 of 8 planted mutations caught.

The one thing I changed in your tests

RunGdnPostConvTokenTileCase could not catch the defect you had to fix.

Your own spec records that the first warp-reduction ordering you tried — summing the four lane-owned squares sequentially — changed production tokens (83fcdc45…1d496ff0…) and was rejected for exactly that. I re-created that rejected arithmetic and pushed it through the bf16 store the test compares on:

  • 0 of 11,160 rows differ
  • 0 of ~1.4M elements differ

The f32 sums do differ (3,580 of 20,000 rows) — but 1/sqrtf moves the result by about 1 ulp f32, and __float2bfloat16 rounds it away. bf16 keeps 8 mantissa bits, so a bf16 CHECK(q1 == q0) is a tolerance gate wearing an exactness costume. It was satisfied by an implementation you already knew was wrong; only your manual e2e token-SHA on sm_120 caught it, and that's neither in CI nor reachable by a reviewer.

Fix applied: the case now takes a DType qkv and sweeps {f32, bf16}, f32 first — the same shape your sibling RunGdnPostConvSplitByteExactCase already uses. With an f32 q_out the mutant fails on 3,580/20,000 rows.

This is worth internalising beyond this PR: whenever a test guards a floating-point reduction order, it needs an F32 output arm. Same trap hides the Tgate = __nv_bfloat16 instantiation, which the case still never reaches because dpacked_ba is f32-only.

New issue #305 — a pre-existing race, not yours

While reading the exact-chunk mapping I found that only chunk 0 reads the initial conv_state (v = srow[width + ti], reachable only when token_offset < width) while only the last chunk writes the final one (srow[j] = v, under token_end == t_len). Those are different, unordered blocks, so they race whenever has_initial_state[s] is set and t_len > kConvExactM. I confirmed both sites on origin/main independently — it's default-ON (VT_CONV_REG + exact chunks) and predates this row, so it got its own issue rather than a silent fix.

Upstream doesn't have this shape: causal_conv1d.py:147-150 keeps the prior-state read and the tl.store in the same Triton program with a debug_barrier() between, and stores to a separate output coordinate.

Related: your channel-tile spec said the accepted arm-2 kernel "preloads both stripes before processing either" as the fix for the stripe-overwrite failure. I corrected that to say it narrows the window rather than closing it — the arm's correctness rests on the initial-state read happening at instruction ~0, not on an ordering guarantee. Your note that a first serial-stripe arm failed the matrix for this reason is actually good evidence the window is observable on that hardware.

Smaller things done while landing

  • Your red agent-record check was just the stale docs/STATUS.md character ratchet — the branch shrank the page 137 chars without lowering STATUS_RATCHET["chars"]. Re-pinned to the measured value.
  • docs/BENCHMARKS.md quoted a 233.955 ms baseline against the 219.506 ms arm-1 figure, which gives 6.18%, not the 6.44% stated everywhere else — 219.506 was measured against 234.605 in the same run. Now quotes the same-run basis and says so.
  • Linked issue RTX 5070 Ti: close Qwen3.5-4B TTFT, TPOT, and VRAM gaps vs vLLM #206 in the roadmap issue table and both specs; it was in neither.
  • Conflicts: benchmark-record.md union-appended (it's genuinely append-only), NOW.md rebuilt from main with only your three scoped edits reapplied. I deliberately did not carry your flock /tmp/gpu GPU-lock bullet or the operator-protocol rewording — main is newer on both, and the former contradicts the local-ai-worker parking convention.

One honest limit: the maintainer box has no GPU and no nvcc, so no CUDA test in this row was executed by me. The portable-header results above were run for real; the CUDA-side claims are host emulation over the same index space plus bitwise arithmetic replay. The modified test compiles clean and skips without CUDA — it still owes a real GPU run on your 5070 Ti, and that's the one thing I'd ask you to confirm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants